components/bank/components/sendBox.tsx (2)

29-38: Consider extracting IBC chain configuration.

The ibcChains array is currently hardcoded with only Osmosis. For better maintainability and scalability, consider:

1. Moving chain configurations to a separate config file
2. Setting selectedChain's initial state to 'osmosis' since it's the only option

- const [selectedChain, setSelectedChain] = useState('');
+ const [selectedChain, setSelectedChain] = useState('osmosis');

---

67-67: Remove unnecessary div wrapper.

The empty div wrapper doesn't provide any styling or structural benefit.

- <div className="">
+ <>

components/bank/components/tokenList.tsx (3)

24-31: Consider using React state for modal control instead of direct DOM manipulation.

The current implementation has two potential improvements:

1. Replace direct DOM manipulation with React state for better control flow
2. Use proper typing for the denom parameter

- const [selectedDenom, setSelectedDenom] = useState<any>(null);
+ const [selectedDenom, setSelectedDenom] = useState<CombinedBalanceInfo['metadata'] | null>(null);
+ const [isModalOpen, setIsModalOpen] = useState(false);

  const openModal = (denom: CombinedBalanceInfo['metadata']) => {
    setSelectedDenom(denom);
-   const modal = document.getElementById('denom-info-modal') as HTMLDialogElement;
-   if (modal) {
-     modal.showModal();
-   }
+   setIsModalOpen(true);
  };

---

34-49: Add aria-label to search input for better accessibility.

The search input should have an aria-label for screen readers.

  <input
    type="text"
    placeholder="Search for a token..."
+   aria-label="Search for a token"
    className="input input-md w-full md:w-64 pr-8 bg-[#0000000A] dark:bg-[#FFFFFF0F]"
    style={{ borderRadius: '12px' }}
    value={searchTerm}
    onChange={e => setSearchTerm(e.target.value)}
  />

---

81-91: Consider extracting number formatting logic into a utility function.

The token amount formatting logic is complex and could be reused elsewhere. Consider moving it to a utility function for better maintainability and reusability.

+ // In utils/format.ts
+ export const formatTokenAmount = (
+   amount: string,
+   exponent: number,
+   display: string
+ ) => {
+   const value = Number(shiftDigits(amount, -exponent));
+   return `${value.toLocaleString(undefined, {
+     maximumFractionDigits: exponent,
+   })} ${truncateString(display, 12).toUpperCase()}`;
+ };

- <p className="font-semibold text-[#161616] dark:text-white">
-   {Number(
-     shiftDigits(
-       balance.amount,
-       -(balance.metadata?.denom_units[1]?.exponent ?? 6)
-     )
-   ).toLocaleString(undefined, {
-     maximumFractionDigits: balance.metadata?.denom_units[1]?.exponent ?? 6,
-   })}{' '}
-   {truncateString(balance.metadata?.display ?? '', 12).toUpperCase()}
- </p>
+ <p className="font-semibold text-[#161616] dark:text-white">
+   {formatTokenAmount(
+     balance.amount,
+     balance.metadata?.denom_units[1]?.exponent ?? 6,
+     balance.metadata?.display ?? ''
+   )}
+ </p>

components/bank/components/historyBox.tsx (3)

36-36: Remove console.log statement.

Debug console.log statements should not be present in production code.

-console.log(metadatas);

---

131-140: Simplify amount formatting logic.

The amount formatting logic is complex and could be simplified by extracting it into a separate function. This would improve readability and maintainability.

+const formatAmount = (amt: { amount: string; denom: string }) => {
+  const metadata = getMetadata(amt);
+  const exponent = Number(metadata?.denom_units[1]?.exponent) || 6;
+  return `${Number(shiftDigits(amt.amount, -exponent)).toLocaleString(undefined, { 
+    maximumFractionDigits: exponent 
+  })} ${formatDenom(amt.denom)}`;
+};

{tx.data.from_address === address ? '-' : '+'}
-{tx.data.amount
-  .map(amt => {
-    const metadata = metadatas?.metadatas.find(m => m.base === amt.denom);
-    const exponent = Number(metadata?.denom_units[1]?.exponent) || 6;
-    return `${Number(shiftDigits(amt.amount, -exponent)).toLocaleString(undefined, { 
-      maximumFractionDigits: exponent 
-    })} ${formatDenom(amt.denom)}`;
-  })
-  .join(', ')}
+{tx.data.amount.map(formatAmount).join(', ')}

---

74-75: Consider adding loading and empty states.

The component should handle loading and empty states gracefully. Currently, there's no visual feedback for these states.

<div className="flex-1 overflow-hidden h-full">
  <div className="h-full overflow-y-auto">
+   {isLoading && (
+     <div className="flex items-center justify-center h-full">
+       <p>Loading transactions...</p>
+     </div>
+   )}
+   {!isLoading && Object.keys(groupedTransactions).length === 0 && (
+     <div className="flex items-center justify-center h-full">
+       <p>No transactions found</p>
+     </div>
+   )}
    {Object.entries(groupedTransactions).map((/* ... */))}

components/groups/components/myGroups.tsx (5)

39-56: Consider optimizing URL synchronization logic

The URL synchronization logic could be improved for better maintainability and performance.

Consider these improvements:

1. Extract group finding logic to a utility function:

+ const findGroupByPolicyAddress = (policyAddress: string, groups: ExtendedQueryGroupsByMemberResponseSDKType['groups']) => {
+   return groups.find(
+     g => g.policies && g.policies.length > 0 && g.policies[0]?.address === policyAddress
+   );
+ };

  useEffect(() => {
    const { policyAddress } = router.query;
    if (policyAddress && typeof policyAddress === 'string') {
-     const group = groups.groups.find(
-       g => g.policies && g.policies.length > 0 && g.policies[0]?.address === policyAddress
-     );
+     const group = findGroupByPolicyAddress(policyAddress, groups.groups);
      if (group) {
        setSelectedGroup({
          policyAddress,
          name: group.ipfsMetadata?.title ?? 'Untitled Group',
          threshold:
            (group.policies[0]?.decision_policy as ThresholdDecisionPolicySDKType)?.threshold ??
            '0',
        });
      }
    }
  }, [router.query, groups.groups]);

---

65-72: Add type safety to event handlers

The handlers are well-implemented, but could benefit from TypeScript improvements.

Consider adding a type for the selected group state:

+ type SelectedGroup = {
+   policyAddress: string;
+   name: string;
+   threshold: string;
+ };

- const [selectedGroup, setSelectedGroup] = useState<{
-   policyAddress: string;
-   name: string;
-   threshold: string;
- } | null>(null);
+ const [selectedGroup, setSelectedGroup] = useState<SelectedGroup | null>(null);

- const handleSelectGroup = (policyAddress: string, groupName: string, threshold: string) => {
+ const handleSelectGroup = (policyAddress: string, groupName: string, threshold: string): void => {

---

113-123: Enhance table accessibility

The table structure could benefit from improved accessibility features.

Consider these accessibility improvements:

- <table className="table w-full border-separate border-spacing-y-3">
+ <table 
+   className="table w-full border-separate border-spacing-y-3"
+   role="grid"
+   aria-label="Groups list"
+ >
    <thead className="sticky top-0 bg-[#F0F0FF] dark:bg-[#0E0A1F]">
      <tr className="text-sm font-medium">
-       <th className="bg-transparent w-1/6">Group Name</th>
+       <th className="bg-transparent w-1/6" scope="col">Group Name</th>
-       <th className="bg-transparent w-1/6">Active proposals</th>
+       <th className="bg-transparent w-1/6" scope="col">Active proposals</th>
        // Apply similar changes to other th elements
      </tr>
    </thead>

---

207-207: Use optional chaining for cleaner code

The current null check can be simplified using optional chaining.

- const policyAddress = (group.policies && group.policies[0]?.address) || '';
+ const policyAddress = group.policies?.[0]?.address || '';

🧰 Tools

🪛 Biome

[error] 207-207: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

---

261-265: Extract balance formatting logic

The balance formatting logic could be extracted to a reusable utility function.

+ const formatBalance = (amount: string): string => {
+   return `${Number(shiftDigits(amount, -6)).toLocaleString(undefined, {
+     maximumFractionDigits: 6,
+   })} MFX`;
+ };

- {Number(shiftDigits(balance?.amount ?? '0', -6)).toLocaleString(undefined, {
-   maximumFractionDigits: 6,
- })}{' '}
- MFX
+ {formatBalance(balance?.amount ?? '0')}

components/groups/forms/proposals/ConfirmationForm.tsx (4)

Line range hint 39-78: Consider improving type safety.

Two potential improvements for type safety:

1. The address prop should be validated before use
2. The customMessage: any type in MessageTypeMap bypasses type checking

Consider this implementation:

- customMessage: any;
+ customMessage: {
+   type: string;
+   value: unknown;
+ };

Also, add address validation:

if (!address || !/^manifest1[a-zA-Z0-9]{38}$/.test(address)) {
  throw new Error('Invalid manifest address format');
}

---

Line range hint 119-159: Refactor message object handling for better separation of concerns.

The getMessageObject function is doing too much. Consider splitting it into smaller, focused functions and improving error handling.

Consider this refactor:

const validateComposedMessage = (
  composedMessage: any, 
  messageType: string
): void => {
  if (!composedMessage?.value) {
    throw new Error(`Invalid message composition for type: ${messageType}`);
  }
  if (!composedMessage.typeUrl || typeof composedMessage.value !== 'object') {
    throw new Error(`Invalid message structure for type: ${messageType}`);
  }
};

const createAnyMessage = (
  composedMessage: { typeUrl: string; value: object }
): Any => {
  try {
    return Any.fromPartial({
      typeUrl: composedMessage.typeUrl,
      value: composedMessage.value,
    });
  } catch (error) {
    throw new Error(`Failed to create Any message: ${error.message}`);
  }
};

const getMessageObject = (
  message: { type: keyof MessageTypeMap } & Record<string, any>
): Any => {
  const composer = messageTypeToComposer[message.type];
  if (!composer) {
    throw new Error(`Unknown message type: ${message.type}`);
  }

  const messageData = prepareMessageData(message);
  const composedMessage = composer(messageData);
  
  validateComposedMessage(composedMessage, message.type);
  return createAnyMessage(composedMessage);
};

---

Line range hint 160-206: Enhance metadata validation and transaction error handling.

The proposal metadata lacks validation, and the transaction handling could be more robust.

Consider these improvements:

const validateProposalMetadata = (metadata: typeof proposalMetadata) => {
  const required = ['title', 'authors', 'summary', 'details'];
  const missing = required.filter(field => !metadata[field]);
  if (missing.length > 0) {
    throw new Error(`Missing required metadata fields: ${missing.join(', ')}`);
  }
};

const handleConfirm = async () => {
  try {
    setIsSigning(true);
    validateProposalMetadata(proposalMetadata);
    
    const CID = await uploadMetaDataToIPFS();
    const messages = await Promise.all(
      formData.messages.map(message => getMessageObject(message))
    );
    
    const msg = cosmos.group.v1.MessageComposer.fromPartial.submitProposal({
      // ... existing props
    });
    
    const fee = await estimateFee(address, [msg]);
    await tx([msg], {
      fee,
      onSuccess: nextStep,
      onError: (error) => {
        console.error('Transaction failed:', error);
        // Handle error appropriately
      }
    });
  } catch (error) {
    console.error('Confirmation failed:', error);
    // Handle error appropriately
  } finally {
    setIsSigning(false);
  }
};

---

207-296: Enhance accessibility and user feedback.

The UI implementation needs accessibility improvements and better loading state feedback.

Consider these improvements:

- <div className="dark:bg-[#2A2A38] bg-[#FFFFFF] p-4 rounded-[12px]">
+ <div 
+   className="dark:bg-[#2A2A38] bg-[#FFFFFF] p-4 rounded-[12px]"
+   role="region"
+   aria-label="Proposal Information"
+ >

- <button onClick={handleConfirm} disabled={isSigning || !address}>
+ <button
+   onClick={handleConfirm}
+   disabled={isSigning || !address}
+   aria-busy={isSigning}
+   aria-label={isSigning ? 'Signing transaction...' : 'Sign Transaction'}
+ >

Also consider extracting color values to CSS variables:

const colors = {
  darkBg: 'var(--dark-bg, #2A2A38)',
  lightBg: 'var(--light-bg, #FFFFFF)',
  // ... other colors
};

components/groups/components/groupProposals.tsx (3)

41-41: Consider using a more type-safe initial state

Instead of using null, consider using undefined for the initial state of selectedProposal. This aligns better with TypeScript's handling of optional values and makes it clearer that the proposal hasn't been selected yet, rather than explicitly being set to null.

- const [selectedProposal, setSelectedProposal] = useState<ProposalSDKType | null>(null);
+ const [selectedProposal, setSelectedProposal] = useState<ProposalSDKType | undefined>(undefined);

---

221-228: Consider extracting modal handling logic into a custom hook

The modal handling logic is duplicated across openInfoModal and openMemberModal. Consider extracting this into a custom hook for better reusability and maintainability.

// useModal.ts
export const useModal = (modalId: string) => {
  const openModal = () => {
    const modal = document.getElementById(modalId) as HTMLDialogElement | null;
    if (modal) {
      modal.showModal();
    } else {
      console.error(`Modal element '${modalId}' not found`);
    }
  };
  return { openModal };
};

// Usage in component:
const { openModal: openInfoModal } = useModal('group-info-modal');
const { openModal: openMemberModal } = useModal('member-management-modal');

Also applies to: 230-237

---

314-337: Extract time calculation logic into a utility function

The time calculation logic is complex and could be reused elsewhere. Consider extracting it into a separate utility function for better maintainability and reusability.

// timeUtils.ts
export const calculateTimeLeft = (endTime: Date): string => {
  const now = new Date();
  const diff = endTime.getTime() - now.getTime();
  
  const msPerMinute = 1000 * 60;
  const msPerHour = msPerMinute * 60;
  const msPerDay = msPerHour * 24;
  
  if (diff <= 0) return 'none';
  if (diff >= msPerDay) {
    const days = Math.floor(diff / msPerDay);
    return `${days} day${days === 1 ? '' : 's'}`;
  }
  if (diff >= msPerHour) {
    const hours = Math.floor(diff / msPerHour);
    return `${hours} hour${hours === 1 ? '' : 's'}`;
  }
  if (diff >= msPerMinute) {
    const minutes = Math.floor(diff / msPerMinute);
    return `${minutes} minute${minutes === 1 ? '' : 's'}`;
  }
  return 'less than a minute';
};

// Usage in component:
const timeLeft = calculateTimeLeft(new Date(proposal?.voting_period_end));

components/groups/forms/proposals/ProposalMessages.tsx (2)

38-120: Consider extracting validation logic into a custom hook.

The validation logic and state management could be moved to a custom hook for better reusability and separation of concerns. This would make the component more maintainable and allow the validation logic to be reused across other components.

Example implementation:

// useMessageValidation.ts
export const useMessageValidation = (messages: Message[]) => {
  const [isMessageValidArray, setIsMessageValidArray] = useState<boolean[]>(
    messages.map(() => false)
  );

  const updateMessageValidity = useCallback((index: number, isValid: boolean) => {
    setIsMessageValidArray(prevState => {
      const newState = [...prevState];
      newState[index] = isValid;
      return newState;
    });
  }, []);

  const checkFormValidity = useCallback(() => {
    return messages.length > 0 && isMessageValidArray.every(isValid => isValid);
  }, [messages.length, isMessageValidArray]);

  return {
    isMessageValidArray,
    updateMessageValidity,
    checkFormValidity
  };
};

---

153-330: Add error handling for edge cases.

While the switch-case complexity is intentional (as per team requirements), consider adding error handling for edge cases such as:

• Invalid message types
• Malformed message objects
• Type conversion errors

Example implementation:

try {
  // Existing switch-case logic
} catch (error) {
  console.error('Error updating message:', error);
  // Handle the error appropriately
  // Consider showing a user-friendly error message
}

Configuration used: CodeRabbit UI
Review profile: CHILL

Learnt from: chalabi2
PR: liftedinit/manifest-app#9
File: components/groups/components/groupProposals.tsx:373-373
Timestamp: 2024-11-06T01:11:11.913Z
Learning: In `components/groups/components/groupProposals.tsx`, all proposals have at least one message, so accessing `proposal.messages[0]` without a length check is safe.

Learnt from: chalabi2
PR: liftedinit/manifest-app#9
File: components/groups/components/groupProposals.tsx:207-208
Timestamp: 2024-11-06T00:51:54.499Z
Learning: In the `GroupProposals` component, redundant checks for `group.policies.length > 0` are unnecessary because prior validations ensure that `group.policies[0]` is safe to access.

Learnt from: chalabi2
PR: liftedinit/manifest-app#9
File: components/groups/components/myGroups.tsx:184-184
Timestamp: 2024-11-06T01:21:36.634Z
Learning: In `components/groups/components/myGroups.tsx`, it's acceptable to pass the entire `selectedGroup` object as the `policyThreshold` prop to the `GroupProposals` component, even if the expected type is `ThresholdDecisionPolicy`.

Learnt from: chalabi2
PR: liftedinit/manifest-app#9
File: components/groups/components/myGroups.tsx:184-184
Timestamp: 2024-11-06T01:21:36.634Z
Learning: In `components/groups/components/myGroups.tsx`, it's acceptable to pass the entire `selectedGroup` object as the `policyThreshold` prop to the `GroupProposals` component, even if the expected type is `ThresholdDecisionPolicy`.

Learnt from: chalabi2
PR: liftedinit/manifest-app#9
File: components/groups/forms/proposals/ProposalMessages.tsx:163-315
Timestamp: 2024-11-06T01:24:11.881Z
Learning: In `components/groups/forms/proposals/ProposalMessages.tsx`, the switch-case statement in the `handleChangeMessage` function is intentionally designed to support specific requests made by the team and should remain as is.

#!/bin/bash
# Verify refetchHistory implementation in SendForm and IbcSendForm
echo "Checking SendForm implementation..."
ast-grep --pattern 'function SendForm({ $$$, refetchHistory, $$$ }) {
  $$$
}'

echo "Checking IbcSendForm implementation..."
ast-grep --pattern 'function IbcSendForm({ $$$, refetchHistory, $$$ }) {
  $$$
}'

- console.log({ messageData });
- console.log({ composedMessage });
+ const logger = process.env.NODE_ENV === 'development' 
+   ? console.log 
+   : () => {};
+ logger({ messageData });
+ logger({ composedMessage });

+ // Add error boundary
+ if (!composer) {
+   throw new Error(`No composer found for message type: ${message.type}`);
+ }